-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Inline plugins (plugins without running VM2) #23443
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's build out the frontend and backend changes on top of this to make sure we don't need anything else changed in the model to minimise number of migrations.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
fd9f9c3
to
13545fc
Compare
b29d269
to
b32c926
Compare
1955e54
to
61bd215
Compare
b84b06f
to
e0029b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super cool. Some really clean work here (especially when working with such a messy problem!) Only one suggestion regarding tests but not blocking.
We might want to be extra careful merging this. I haven't tried it locally so worth triple checking and if there is any doubts doing a canary style deploy (with support from infra)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was expecting to find a test for this tbh. At least one happy path test per inline plugin would be great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the tests from the plugin repo and put them in inline.test.ts
(the semvar-flattener
one) - does this cover the kind of testing you mean, or something beyond that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's what he meant, but seeing a functional test that actually calls an inline plugin would be nice (maybe I'm missing one?)
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
889c186
to
05aa95a
Compare
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ben White <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ben White <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ben White <[email protected]>
Problem
Plugins run inside a VM, which incurs memory and cpu overhead, and code complexity. As many plugins are now considered trusted-source in cloud environments, we can inline the code for commonly used plugins instead, focusing for now on processEvent plugins, to get some improved performance in the ingestion pipeline.